Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for vote “goal” placeholders to the Bukkit PlaceholderAPI expansion, enabling display of the current vote goal target and progress fetched from the Azuriom API.
Changes:
- Introduces
%azlink_vote_goal_target%and%azlink_vote_goal_progress%placeholders. - Extends the vote API response model to include a
goalobject (target/progress). - Wires goal parsing + placeholder evaluation into
VotePlaceholderProvider.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.plugin.getPlugin() | ||
| .getHttpClient() | ||
| .request(HttpClient.RequestMethod.GET, "/vote/azlink?usernames=" + names, null, VoteResponse.class) | ||
| .thenAccept(response -> { | ||
| this.pendingRefresh = false; | ||
|
|
||
| this.voteSites.clear(); | ||
| this.users.clear(); | ||
| this.topVotes.clear(); | ||
| this.goal = response.goal; | ||
|
|
There was a problem hiding this comment.
The HTTP request completes on the async executor (HttpClient.request uses supplyAsync), but the thenAccept callback mutates shared state (voteSites/users/topVotes/goal) that is also read from placeholder evaluation on the server thread. This creates a real data-race risk (e.g., inconsistent reads or IndexOutOfBounds/visibility issues). Consider marshalling the state update back onto the Bukkit main thread (scheduler runTask) or swapping in immutable snapshots / concurrent collections for all shared fields (not just goal).
| if (currentGoal == null) { | ||
| return "0"; | ||
| } | ||
|
|
||
| switch (parts[1]) { | ||
| case "target": | ||
| return Integer.toString(currentGoal.target); | ||
| case "progress": | ||
| return Integer.toString(currentGoal.progress); |
There was a problem hiding this comment.
goalPlaceholder returns "0" whenever goal is null, even if the requested sub-key is invalid (e.g., %azlink_vote_goal_typo% would yield 0 instead of returning null). This makes invalid placeholders harder to detect and is inconsistent with other placeholder handlers. Consider switching on parts[1] first and only returning "0" for the known keys (target/progress) when goal is missing, while still returning null for unknown keys.
| if (currentGoal == null) { | |
| return "0"; | |
| } | |
| switch (parts[1]) { | |
| case "target": | |
| return Integer.toString(currentGoal.target); | |
| case "progress": | |
| return Integer.toString(currentGoal.progress); | |
| String key = parts.length > 1 ? parts[1] : ""; | |
| switch (key) { | |
| case "target": | |
| return currentGoal != null ? Integer.toString(currentGoal.target) : "0"; | |
| case "progress": | |
| return currentGoal != null ? Integer.toString(currentGoal.progress) : "0"; |
No description provided.